-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Backports release 1.12 #57536
base: release-1.12
Are you sure you want to change the base?
Backports release 1.12 #57536
Conversation
…ional` (#55853) Relying on `ScopedValues`, set `BigFloat` precision without mutating the global default, while constructing `Rational` from `AbstractIrrational`. Also helps avoid reading the global defaults for the precision and rounding mode, together with #56095. What does this fix: * in the case of the `Irrational` constants defined in `MathConstants`: relevant methods have `@assume_effects :foldable` applied, which includes `:effect_free`, which requires that no globals be mutated (followup on #55886) * in the case of `AbstractIrrational` values in general, this PR prevents data races on the global `BigFloat` precision (cherry picked from commit 2a89f71)
Reimplement all of the trim verification support in Julia as a compiler analysis pass. Move all this verification code into the Compiler julia code, where we have much better utilities for pretty printing and better observability for analysis. (cherry picked from commit 1045bd8)
Recompute some O(n) things a bit less on every statement. Fix an assert that ensured LimitedAccuracy does not accidentally get preserved when it should have been deleted: the (local) cache should not contain things that are marked as dead (RIP), as that was leading to much code not getting cached when it logically should have. Simplify the computation of frame_parent when the cycle_parent isn't needed. (cherry picked from commit 0366a2a)
- disable load path setup - remove upstreamed change to utf8proc_map - fix warning for accessing mod.main in the wrong world - remove binding backedges when trimming (cherry picked from commit d77c24f)
This change removes an inconsistency between `>:` and `<:`. We were parsing `where {A>:B>:C}` forms, but not recognizing them in lowering. Before: ``` julia> Vector{T} where Int<:T<:Number Vector{T} where Int64<:T<:Number julia> Vector{T} where Number>:T>:Int ERROR: syntax: invalid bounds in "where" around REPL[14]:1 ``` After: ``` julia> Vector{T} where Number>:T>:Int Vector{T} where Int64<:T<:Number ``` (cherry picked from commit 2e57730)
This restores 1.11 behavior. However, I find this function a bit problematic, since it is asking whether the binding is `constant` in a particular sense, but not whether it is `const` (in the sense of having been declared with the keyword or equivalent), so the shortening is confusing. However, we don't need to address that now. Fixes #57475. (cherry picked from commit 1ff98a5)
We have historically allowed the following without warning or error: ``` const x = 1 x = 1 ``` As well as ``` const x = 1 x = 2 ``` with a UB warning. In 1.12, we made the second case error, as part of the general binding partition changes, but did not touch the first case, because it did not have the warning. I think this made a reasonable amount of sense, because we essentially treated constants as special kinds of mutable globals (to which assignment happened to be UB). However, in the 1.12+ design, constants and globals are quite sepearate beasts, and I think it makes sense to extend the error to the egal case also, even if it is technically more breaking. In fact, I already thought that's what I did when I implemented the new effect model for global assignment, causing #57566. I can't think of a legitimate reason to keep this special case. For those who want to do binding replacement, the `const` keyword is mandatory, so the assignment is now literally always a semantic no-op or an error. Fixes #57566. (cherry picked from commit 8f00a51)
This is my attempt to resolve #53000. ``` julia> solve ERROR: UndefVarError: `solve` not defined in `Main` Suggestion: check for spelling errors or missing imports. Hint: a global variable of this name also exists in CommonSolve. - Also exported by SciMLBase. - Also exported by DiffEqBase. - Also exported by JumpProcesses. - Also exported by LinearSolve. - Also exported by BracketingNonlinearSolve (loaded but not imported in Main). - Also exported by SimpleNonlinearSolve. - Also exported by NonlinearSolve. - Also exported by OrdinaryDiffEqLowStorageRK (loaded but not imported in Main). - Also exported by OrdinaryDiffEqSSPRK (loaded but not imported in Main). - Also exported by OrdinaryDiffEqVerner (loaded but not imported in Main). - Also exported by OrdinaryDiffEqBDF (loaded but not imported in Main). - Also exported by OrdinaryDiffEqTsit5 (loaded but not imported in Main). - Also exported by OrdinaryDiffEqRosenbrock (loaded but not imported in Main). - Also exported by OrdinaryDiffEqDefault (loaded but not imported in Main). - Also exported by Sundials. ``` I would have beefed up the test case, but can't follow how it works. --------- Co-authored-by: Alex Arslan <[email protected]> Co-authored-by: Ian Butterworth <[email protected]> Co-authored-by: Jameson Nash <[email protected]> (cherry picked from commit 5f13cd2)
These were the intended semantics of #57311 (and matches what it used to do in 1.11). Note however that this differs from the body-ful form, which now always tries to extend. Fixes #57546. Note that this implementation is slightly inefficient since it goes through a binding replacement. However, there's a change coming down the line which optimizes these replacements. I do think it should eventually be refactored to just create the binding directly, but that's a little bit of a larger change. (cherry picked from commit 7fa0c13)
…gler (#57579) In the line of C code: ```C const int64_t timeout = jl_options.timeout_for_safepoint_straggler_s * 1000000000; ``` `jl_options.timeout_for_safepoint_straggler_s` is an `int16_t` and `1000000000` is an `int32_t`. The result of `jl_options.timeout_for_safepoint_straggler_s * 1000000000` will be an `int32_t` which may not be large enough to hold the value of `jl_options.timeout_for_safepoint_straggler_s` after converting to nanoseconds, leading to overflow. (cherry picked from commit 9cafd35)
The `_apply_pure` function only has one user, and that user is unsound anyways and should not be used, so replace that with equivalent `_call_in_world_total` call and remove unnecessary definitions. The awkward distinction between `invokelatest` and `_call_latest` has not been relevant (and indeed causes performance issues) since kwfunc was introduced in #47157. (cherry picked from commit 671c6a1)
--------- Co-authored-by: Kristoffer <[email protected]> Co-authored-by: Ian Butterworth <[email protected]> (cherry picked from commit 0dd78f7)
This repeats the exercise in #57405. This is required for correctness, because the ->deprecated flag also affects `using` resolution (since it makes the tagged binding weaker for `using` purposes). That said, in general our `->deprecated` semantics have been somewhat underspecified with lots of `XXX` comments in the surrounding code. This tries to define the semantics to give a depwarn on *access* (read or write) when: 1. Either the binding itself is deprecated; or 2. The implicit imports pass through a deprecated binding. However, we do not give depwarns on access to bindings that were explicitly imported (although we do give those warnings on the import) - the reasoning being that it's the import that needs to be adjusted not the access. Additionally, this PR moves into the direction of making the depwarn a semantic part of the global access, by adjusting codegen and inference appropriately. (cherry picked from commit 1a3cbb1)
When inference and codegen queries the partitions of a binding, they often only care about a subset of the information in the partition. At the moment, we always truncate world ranges to the most precise partition that contains the relevant query. However, we can instead expand the world range to cover all partitions that are equivalent for the given query. To give a concrete example, both inference and codegen never care about the exported flag in a binding partition, so we should not unnecessarily truncate a world range just because an export was introduced. Further, this commit lays the ground work to stop invalidating code for these same kinds of transitions, although the actual logic to do that will come in a separate PR. (cherry picked from commit a5157c0)
This renames the partition flags to start with PARTITION_ and turns the Binding flags into a bitfield that can be accessed atomically in preparation for adding further flags. (cherry picked from commit a802faf)
Our implicit edge tracking for bindings does not explicitly store any edges for bindings in the *current* module. The idea behind this is that this is a good time-space tradeoff for validation, because substantially all binding references in a module will be to its defining module, while the total number of methods within a module is limited and substantially smaller than the total number of methods in the entire system. However, we have an issue where the code that stores these edges and the invalidation code disagree on which module is the *current* one. The edge storing code was using the module in which the method was defined, while the invalidation code was using the one in which the MethodTable is defined. With these being misaligned, we can miss necessary invalidations. Both options are in principle possible, but I think the former is better, because the module in which the method is defined is also the module that we are likely to have a lot of references to (since they get referenced implicitly by just writing symbols in the code). However, this presents a problem: We don't actually have a way to iterate all the methods defined in a particular module, without just doing the brute force thing of scanning all methods and filtering. To address this, build on the deferred scanning code added in #57615 to also add any scanned modules to an explicit list in `Module`. This costs some space, but only proportional to the number of defined methods, (and thus proportional to the written source code). Note that we don't actually observe any issues in the test suite on master due to this bug. However, this is because we are grossly over-invalidating, which hides the missing invalidations from this issue (#57617). (cherry picked from commit 274d80e)
@nanosoldier |
1.12 branch version of #57679
Co-authored-by: Jishnu Bhattacharya <[email protected]> (cherry picked from commit 3e57a8a)
Fixes #57702. We're calling cl-convert- on `using` and `import` statements when we shouldn't, so if there's a nearby local that gets boxed (recursive function definition in this case), and the local shares a name with something in an import statement, we get a box access where we want a raw symbol. Before: ``` julia> let; let; import SHA: R; end; let; R(x...) = R(x); end; end ERROR: TypeError: in import, expected Symbol, got a value of type Expr Stacktrace: [1] top-level scope @ REPL[1]:1 ``` After: ``` julia> let; let; import SHA: R; end; let; R(x...) = R(x); end; end (::var"#R#R##0") (generic function with 1 method) ``` Previously, symbols in `import`/`using` statements would be wrapped with `outerref`, which cl-convert- wouldn't peek into. This protected us from this problem in 1.11. (cherry picked from commit ca17927)
(cherry picked from commit e14fb0c)
This was an oversight. (cherry picked from commit 17f99bc)
Make sure that non-ASCII in the big string macro correctly throws an ArgumentError instead of a string indexing error. (cherry picked from commit 246c93b)
When getting stacktraces on non-X86 platforms, the first frame may not have been set up yet, incorrectly triggering this bad-frame detection logic. This should fix the issue of async unwind failing after only getting 2 frames, if the first frame happens to land in the function header. This is not normally an issue on X86 or non-signals, but also causes no expected issues to be the same logic there too. Fix #52334 After (on arm64-apple-darwin24.3.0): ``` julia> f(1) Warning: detected a stack overflow; program state may be corrupted, so further execution might be unreliable. ERROR: StackOverflowError: Stacktrace: [1] f(x::Int64) @ Main ./REPL[3]:1 [2] g(x::Int64) @ Main ./REPL[4]:1 --- the above 2 lines are repeated 39990 more times --- [79983] f(x::Int64) @ Main ./REPL[3]:1 ``` n.b. This will not fix and is not related to any issues where profiling gets only a single stack frame during profiling of syscalls on Apple AArch64. This fix is specific to the bug where it gets exactly 2 frames. (cherry picked from commit f82917a)
Widening from Float32 to Float64 and then rounding to Float16 will not introduce any error, but going from Float64 -> Float32 -> Float16 will round incorrectly if the intermediate Float32 is halfway between two Float16s. Fixes #57805. Thanks to @vtjnash for suggesting the fix. Co-authored-by: Jameson Nash <[email protected]> (cherry picked from commit a676b12)
…simage (#57656) This is quite tricky to test unfortunately, but #57544 caught this and this fixes that --------- Co-authored-by: Jameson Nash <[email protected]> (cherry picked from commit bf01638)
…HS to be global, and prohibit "const x[] = 1" (#57470) The changes to `const` resulted in confusing error messages when it was used inside a function (#57334). On 1.11.3: ``` julia> function f() const x = 1 end ERROR: syntax: unsupported `const` declaration on local variable around REPL[1]:2 Stacktrace: [1] top-level scope @ REPL[1]:1 ``` On nightly: ``` julia> function f() const x = 1 end ERROR: syntax: World age increment not at top level Stacktrace: [1] top-level scope @ REPL[1]:1 ``` In prior versions, we also accepted confused expressions like: ``` x = Ref(1) const x[] = 1 ``` This change adds a new error messages explicitly prohibiting `const` where the left hand side is not declaring variables: ``` ERROR: syntax: `const` left hand side "x[]" contains non-variables around REPL[2]:1 Stacktrace: [1] top-level scope @ REPL[2]:1 ``` Finally, #54773 made `const` stop participating in scope resolution (the left hand side was always taken to be in global scope). Some expressions that were prohibited started being accepted: In 1.11.3: ``` julia> let const x = 1 end ERROR: syntax: unsupported `const` declaration on local variable around REPL[1]:2 Stacktrace: [1] top-level scope @ REPL[1]:1 ``` Nightly: ``` julia> let const x = 1 end 1 ``` This change rejects `const` unless the variable would be in global scope (`global const` would be required in the example), so we don't lose the ability to make `const` in local scope meaningful later. (cherry picked from commit fb01f91)
With the "classic" inference timing disabled, PrecompileTools and SnoopCompile are non-functional on 1.12 & master. #57074 added timing data to the CodeInstances themselves, which should help restore SnoopCompile. However, without the tree structure provided by the old inference timing system, we still need a way to tag MethodInstances that get inferred inside `PrecompileTools.@compile_workload` blocks. This adds a simple mechanism modeled after `@trace_compile` that switches to tagging MethodInstances in `jl_push_newly_inferred`. JuliaLang/PrecompileTools.jl#47 contains (or will contain) the corresponding changes to PrecompileTools.jl. (cherry picked from commit c89b1ff)
Restricts the dispatch for these macros so that only `String` (literal) arguments are accepted: * `int128_str` * `uint128_str` * `big_str` * `ip_str` from the Sockets stdlib * `dateformat_str` from the Dates stdlib Some of these changes make the code in the sysimage less vulnerable to invalidation. (cherry picked from commit f9365a5)
We could make this have a nicer API but for now lets just do what we've done for pkgimages/sysimages and use the envar (cherry picked from commit 17fff87)
Both `--trim=no` and `--trim=safe` are supposed to be safe options by default, so let's not get overzealous and throw out all the IR even though we're not trimming Allows `juliac.jl` to use/test the sysimage creation pipeline unrelated to `--trim` support, as in #57656 (comment) (cherry picked from commit f9e5af1)
Although a package is loaded, we may not have top-level `using` rights. Instead, we need to pull the Module straight out of the loaded modules array in order to do these hacks. (cherry picked from commit 1cead2b)
As recommended in #57589 (comment) (cherry picked from commit eba2a33)
Adds 4 new Float16 fields to CodeInstance to replace Compiler.Timings with continually collected and available measurements. Sample results on a novel method signature: julia> @time code_native(devnull, ÷, dump_module=false, (Int32, UInt16)); 0.006262 seconds (3.62 k allocations: 186.641 KiB, 75.53% compilation time) julia> b = which(÷, (Int32, UInt16)).specializations[6].cache CodeInstance for MethodInstance for div(::Int32, ::UInt16) julia> reinterpret(Float16, b.time_infer_self) Float16(0.0002766) julia> reinterpret(Float16, b.time_infer_total) Float16(0.00049) julia> reinterpret(Float16, b.time_infer_cache_saved) Float16(0.02774) julia> reinterpret(Float16, b.time_compile) Float16(0.003773) Closes #56115 (cherry picked from commit 18b5d8f)
Otherwise inspecting it from julia will be wrong/UB (cherry picked from commit 0df5ad7)
Add an extra indirection to our printing code for `@assert` so that that inferrability is restored after the override. Also partially revert d77c24f which, deleted init code important for JLL's ~This still requires that we bypass this inference-disabling code in JLLWrappers.jl:~ ```julia if isdefined(Base, :Experimental) && isdefined(Base.Experimental, Symbol("@compiler_options")) @eval Base.Experimental.@compiler_options compile=min optimize=0 infer=false end ``` (cherry picked from commit d369c10)
3 interrelated fixes here: - Re-word the confusing error message for a circular dependency error. - Calling `using` inside a package was previously declared to be an error, but never really enforced, so allow it explicitly to do what it logically means (when appearing explicitly, such as from having `Requires.@require` in an `__init__` function). Remove the associated warning in the docs, since this gotcha now simply means what users think it would mean. - A missing lock acquire in create_expr_cache lead to a confusing discrepancy in errors related to the previous two fixes between incremental and serial modes. Fixes #56742 (cherry picked from commit 7f8ca29)
Should prevent invalidation stemming from `MethodInstance`s for * `readline()` * `readlines()` * `Base.run_fallback_repl(::Bool)` * `Base.repl_main(::Any)` * `Base._start()` In each case the issue is the use of the nonconcretely-typed global variable `stdin::IO`. (cherry picked from commit eed18bd)
The `fptrunc`/`fpext` intrinsics were modified in #57160 to throw on non-float arguments. - The arithmetic and math float intrinsics now require all their arguments to be floats - `fptosi`/`fptoui` require their source to be a float - `sitofp`/`uitofp` require their destination type to be a float Also fixes #57384. (cherry picked from commit 0bcc9cd)
…7845) The unsafe_load code tries to reuse some of the logic from safe loads, but needs to be careful to override the parts that are not safe with slower versions of them. Similar to the typo-fix from ec3c02a on v1.11-backports. Seen in the IR for code_llvm(optimize=false, raw=true, unsafe_load, (Ptr{Tuple{UInt128}},)) causing LightBSON to fail. Fix ancapdev/LightBSON.jl#37 (cherry picked from commit 36b046d)
The `_unsetindex!(::GenericMemoryRef)` function was being concretely evaluated (meaning the code was actually being run during inference) by the `REPLInterpreter`. The root cause is that the Base compiler system forgot to mark `atomic_pointerset` as "effect-free". This is simply a bug in the effect system. It didn't cause problems during normal Base compilation because `atomic_pointerset` was correctly marked as "consistent" (concrete evaluation requires both "consistent" and "effect-free"). However, this was still a pretty risky situation. The reason this only caused problems with REPL completion is that the `REPLInterpreter` intentionally ignores the "consistent" requirement under certain conditions to achieve better completion accuracy. This is usually fine, but it relies on "effect-free" being correctly marked. So, when there's a critical bug like this in the effect system, these kinds of dangerous issues can occur. As discussed with Jameson earlier, the effects of atomic pointer operations are not precisely defined. This commit includes the minimal changes necessary to fix #57780, but a more extensive audit is planned for later. - closes #57780 --------- Co-authored-by: Jameson Nash <[email protected]> (cherry picked from commit 6a32f7a)
When we explicitly import a binding (via either `using M: x` or `import`), the corresponding bpart representation is a direct pointer to the imported binding. This is necessary, because that is the precise semantic representation of the import. However, for implicit imports (those created via `using M` without explicit symbol name), the situation is a bit different. Here, there is no semantic content to the particular binding being imported. Worse, the binding is not necessarily unique. Our current semantics are essentially that we walk the entire import graph (both explicit and implicit edges) and if all reachable terminal nodes are either (i) the same binding or (ii) constant bindings with the same value, the import is allowed. In this, we are supposed to ignore cycles, although the current implementation has some trouble with this (#57638, #57699). If the import succeeds, in the current implementation, we then record an arbitrary implicit import edge as in the BindingPartition. In essence, we are creating a spanning tree of the import graph (formed from both the implicit and explicit import edges). However, dynamic algorithms for spanning tree maintenance are complicated and we didn't implement any. As a result, it is possible for later edge additions to accidentally introduce cycles (causing #57699). An additional problem, even if we could keep a consistent spanning tree, is that the spanning tree is not unique. In practice, this is not supposed to be observable, but it is still odd to have a non-unique representation for a particular set of imports. That said, we don't really need the spanning tree. The only place that actually uses it is `which(::Module, ::Symbol)` which is not performance sensitive and arguably a bad API for the above reason that the answer is ill-defined. With all these problems, let's just get rid of the spanning tree all together - as mentioned we don't really need it. Instead, we split the PARTITION_KIND_IMPLICIT into two, one for each of the two cases (importing a global binding, or importing a particular constant value). This is actually a more efficient implementation for the common case of needing to follow a lookup - we no longer need to follow all the import edges. In exchange, more work needs to be done during binding replacement to re-scan the entire import graph. This is probably the right trade-off though, since binding replacement is already a slow path. Fixes #57638, fixes #57699, fixes #57641, fixes #57700, fixes #57343 (cherry picked from commit 60a9f69)
@nanosoldier |
Backported PRs:
Base.summarysize
forMemory
withUnion
#57508Rational
fromAbstractIrrational
#55853circshift!(::AbstractVector, ::Integer)
#57539>:
inwhere
#57554isconst(::GlobalRef)
#57563jl_init__threading
andjl_init_with_image__threading
#57561function
declaration implicitlyglobal
#57562Base.Precompilation.ExplicitEnv
: handle type instability better in constructor #57599nextind
,prevind
methods #57608AnnotatedString
: add concrete type asserts toisvalid
,ncodeunits
#57607Base
:power_by_squaring
: don't requireone
#57590printvalue
#57584OncePerX
more aggressively #57660force_enable_inference
to override Module-level inference settings #57653abstract_apply
: declare type of two closure captures #57684Base
:append!
,resize!
: convert length toInt
before passing it on #57585_lift_svec_ref
: improve type stability #57633string(::LazyLibraryPath)
#57721Tuple
instead ofVector
#57734Core.SimpleVector
intypejoin_union_tuple
#57631::SimpleVector
in_collapse_repeated_frames
#57691import
orusing
#57774!
in a strip-metadata case #57778::Real
fallback stack overflow #57790String
#57781const
inside a function, require the LHS to be global, and prohibit "const x[] = 1" #57470--strip-ir
when compiling with--trim=no
#57659String
#57781Int
to prevent invalidation #57848Need manual backport:
reinit_ccallable
#56987Contains multiple commits, manual intervention needed:
const
inside a function, require the LHS to be global, and prohibit "const x[] = 1" #57470Non-merged PRs with backport label:
Base
:macro cmd
: restrict argument toString
#57862Base
: restrict argument of macrokwdef
toExpr
#57861Compiler
:abstract_eval_invoke_inst
: type assertInt
#57860Compiler
:walk_to_defs
: type assertInt
before calling:
#57859mod
for mixes ofSigned
andUnsigned
#57853macro
methods for pre-compilation #57833@nospecialize
forstring_index_err
#57604maxthreadid
fromThreads
#57490